Repository navigation
Add typed JSON output to theme package - #8683
gonzaloriestra wants to merge 5 commits into
Conversation
9dce70c to
cbf4e74
Compare
cbf4e74 to
8290b55
Compare
8290b55 to
195f2b0
Compare
| vi.mock('@shopify/cli-kit/node/analytics', () => ({ | ||
| recordEvent: vi.fn(), | ||
| compileData: vi.fn().mockReturnValue({timings: {}, errors: {}, retries: {}, events: {}}), | ||
| })) |
There was a problem hiding this comment.
you don't need these special cases no? Just
vi.mock('@shopify/cli-kit/node/ui')
vi.mock('@shopify/cli-kit/node/analytics')
Should work fine
There was a problem hiding this comment.
Updated, thanks
isaacroldan
left a comment
There was a problem hiding this comment.
Minor comment on the tests
195f2b0 to
28d575f
Compare
28d575f to
29801b7
Compare
5f102b2 to
72d4123
Compare
901c0bc to
d5ad578
Compare
d5ad578 to
f62547a
Compare
7fd0bc6 to
515529c
Compare
515529c to
b8db95e
Compare
cf702b0 to
3034081
Compare
3034081 to
9aa6e95
Compare
9b568c8 to
86c3847
Compare
86c3847 to
68b94cf
Compare
68b94cf to
0169d92
Compare
0169d92 to
5c64f25
Compare
|
| Changeset | Package |
|---|---|
theme-check-json-contract.md |
'@shopify/cli': major |
theme-duplicate-json-contract.md |
'@shopify/cli': major |
theme-info-json-contract.md |
'@shopify/cli': major |
theme-list-json-contract.md |
'@shopify/cli': major |
theme-open-json-contract.md |
'@shopify/cli': major |
theme-preview-json-contract.md |
'@shopify/cli': major |
theme-profile-environments.md |
'@shopify/cli': major |
theme-push-json-contract.md |
"@shopify/cli": major |
Differences in type declarationsWe detected differences in the type declarations generated by Typescript for this branch compared to the baseline ('main' branch). Please, review them to ensure they are backward-compatible. Here are some important things to keep in mind:
New type declarationsWe found no new type declarations in this PR Existing type declarationspackages/cli-kit/dist/public/node/base-command.d.ts@@ -35,6 +35,7 @@ declare abstract class BaseCommand extends Command {
argv: string[];
}>;
protected environmentsFilename(): string | undefined;
+ protected validateNonTTYFlags(flags: FlagOutput): void;
protected failMissingNonTTYFlags(flags: FlagOutput, requiredFlags: string[]): void;
private failMissingNonTTYFlagRequirements;
private applicableNonTTYFlagRequirements;
packages/cli-kit/dist/public/node/themes/theme-manager.d.ts@@ -11,5 +11,12 @@ export declare abstract class ThemeManager {
findOrCreate(name?: string, role?: Role): Promise<Theme>;
fetch(name?: string, role?: Role): Promise<Theme | undefined>;
generateThemeName(context: string): string;
- create(themeRole?: Role, themeName?: string): Promise<Theme>;
+ create(themeRole?: Role, themeName?: string): Promise<{
+ createdAtRuntime: boolean;
+ id: number;
+ name: string;
+ processing: boolean;
+ role: string;
+ src?: string;
+ }>;
}
\ No newline at end of file
|
isaacroldan
left a comment
There was a problem hiding this comment.
Found one false success receipt.
🤖 Generated by the review-pr skill
| async command(flags: PackageFlags) { | ||
| await packageTheme(flags.path) | ||
| const result = await packageTheme(flags.path) | ||
| renderThemePackageResult(result, flags.json ? 'json' : 'text') |
There was a problem hiding this comment.
Can we make the archive writer reject output errors before emitting this receipt? With a valid settings_schema.json and an existing Dawn.zip directory, --json returns {path: ".../Dawn.zip"} and exits 0, but no ZIP was written. I checked this with the real writer and files. zip() currently resolves when the output stream closes even when opening it fails. This needs to use the fatal JSON error path.
🤖 Generated by the review-pr skill
WHY are these changes introduced?
Packaging needs a machine-readable artifact receipt.
WHAT is this pull request doing?
Return a strict JSON artifact result with a validated absolute path to the ZIP actually written, while preserving the text presenter.
The public output follows
docs/cli/json-output.md; explicitly named environments use one{environments: [...]}object.How to manually test your changes?
shopify theme package --path . --json shopify theme package --json-schemaChecklist
patchfor bug fixes ·minorfor new features ·majorfor breaking changes) and added a changeset withpnpm changeset add